-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Programming exercises
: Add R programming language template
#9256
Conversation
Programming exercises
: Add R programming language template
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
3fdade7
to
6b618c6
Compare
WalkthroughThis pull request introduces support for the R programming language across various components of the application. It updates the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Files ignored due to path filters (2)
src/main/resources/config/application.yml
is excluded by!**/*.yml
src/main/resources/templates/aeolus/r/default.yaml
is excluded by!**/*.yaml
Files selected for processing (23)
- docs/user/exercises/programming-exercise-features.inc (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (3 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/resources/templates/aeolus/r/default.sh (1 hunks)
- src/main/resources/templates/gitlabci/r/regularRuns/.gitlab-ci.yml (1 hunks)
- src/main/resources/templates/jenkins/r/regularRuns/pipeline.groovy (1 hunks)
- src/main/resources/templates/r/exercise/DESCRIPTION (1 hunks)
- src/main/resources/templates/r/exercise/NAMESPACE (1 hunks)
- src/main/resources/templates/r/exercise/R/convert.R (1 hunks)
- src/main/resources/templates/r/readme (1 hunks)
- src/main/resources/templates/r/solution/DESCRIPTION (1 hunks)
- src/main/resources/templates/r/solution/NAMESPACE (1 hunks)
- src/main/resources/templates/r/solution/R/convert.R (1 hunks)
- src/main/resources/templates/r/test/DESCRIPTION (1 hunks)
- src/main/resources/templates/r/test/tests/testthat.R (1 hunks)
- src/main/resources/templates/r/test/tests/testthat/test-convert.R (1 hunks)
- src/main/webapp/app/entities/programming/programming-exercise.model.ts (1 hunks)
- src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts (1 hunks)
Additional context used
Path-based instructions (9)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Learnings (5)
src/main/resources/templates/aeolus/r/default.sh (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/resources/templates/aeolus/rust/default.sh:14-24 Timestamp: 2024-07-01T10:48:58.558Z Learning: The `default.sh` script in the `src/main/resources/templates/aeolus/rust/` directory is auto-generated by Aeolus and should not be manually edited. Changes need to be made in the Aeolus generator settings or templates.
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/gitlabci/GitLabCIProgrammingLanguageFeatureService.java:24-24 Timestamp: 2024-06-29T14:49:34.001Z Learning: Static code analysis for Rust in GitLab CI will be added in a follow-up PR.
src/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#9074 File: src/main/java/de/tum/in/www1/artemis/service/programming/TemplateUpgradePolicyService.java:35-35 Timestamp: 2024-07-16T20:00:50.653Z Learning: The `defaultRepositoryUpgradeService` in the `TemplateUpgradePolicyService` is used as a default for all non-JAVA languages, with specialized services added based on testing outcomes if necessary.
src/main/resources/templates/gitlabci/r/regularRuns/.gitlab-ci.yml (1)
Learnt from: magaupp PR: ls1intum/Artemis#8859 File: src/main/resources/templates/gitlabci/javascript/regularRuns/.gitlab-ci.yml:36-49 Timestamp: 2024-07-16T22:00:10.722Z Learning: In the Artemis project, the `gradle run` command in the JavaScript CI configuration is used to operate a Java-based notification plugin.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/jenkins/JenkinsProgrammingLanguageFeatureService.java:43-43 Timestamp: 2024-06-29T14:47:20.796Z Learning: Jenkins support for Rust was added in commit d0fb7032e70c001dc8b2c4d0a391ca8bf7e7d586.
Additional comments not posted (41)
src/main/resources/templates/r/exercise/NAMESPACE (1)
1-1
: LGTM! The export pattern is well-defined and serves an important purpose.The
exportPattern("^[^\\.]")
line effectively controls the visibility of module components by exporting only entities whose names do not start with a dot (.
). This pattern is crucial for:
- Enhancing encapsulation and promoting better organization of code.
- Filtering out private or internal entities that typically begin with a dot.
- Allowing developers to manage dependencies and visibility more effectively.
By implementing this export pattern, the module ensures that only appropriately named entities are accessible to other modules or components that import this namespace.
src/main/resources/templates/r/solution/NAMESPACE (1)
1-1
: LGTM! The exported pattern enhances modularity and maintainability.The
exportPattern("^[^\\.]")
line effectively controls the visibility of elements within the module or namespace. By ensuring that only elements not starting with a dot (.
) are exported, it encapsulates internal logic while exposing only the necessary components to other parts of the application.This pattern provides several benefits:
- It prevents unintended access to internal elements that are not meant to be public.
- It enhances modularity by clearly defining the public interface of the module.
- It improves maintainability by reducing the risk of external code relying on internal implementation details.
Great job in implementing this pattern to manage the scope of variables, functions, or classes in the R solution template!
src/main/resources/templates/r/exercise/DESCRIPTION (7)
1-1
: LGTM!The package name "assignment" is appropriate for a student assignment and follows R package naming conventions.
2-2
: LGTM!The package title clearly indicates that this is an R assignment for the Artemis platform.
3-3
: LGTM!The version number 0.0.0.9000 follows semantic versioning and indicates an initial development version, which is appropriate for a new package.
4-4
: LGTM!Listing "Artemis" as the author is suitable for an assignment created by the Artemis platform.
5-5
: LGTM!The description concisely states the purpose of the package as a student assignment, which provides clarity on its educational purpose.
6-6
: LGTM!The MIT license is a permissive open-source license, which is a good choice for an educational assignment as it allows for broad usage and distribution.
7-7
: LGTM!The UTF-8 encoding ensures compatibility with a wide range of characters, which is important for internationalization and proper display of text.
src/main/resources/templates/r/solution/DESCRIPTION (1)
1-7
: LGTM!The
DESCRIPTION
file follows the standard format for an R package and includes all the necessary metadata. The chosen license (MIT) and encoding (UTF-8) are appropriate for the package's educational purpose and ensure broad compatibility.This file is essential for the proper functioning and distribution of the R package within the Artemis platform.
src/main/resources/templates/r/test/DESCRIPTION (1)
1-14
: LGTM!The
DESCRIPTION
file for thetest
package is well-structured and follows the standard format for R packages. The metadata, dependencies, and configuration are clearly defined and appropriate for the package's purpose of testing student assignments within the Artemis framework.The dependencies on the
assignment
package and thetestthat
testing framework are correctly specified, ensuring that the package can effectively test student assignments and leverage standard testing practices.Overall, the file sets up a solid foundation for the
test
package to function effectively within the Artemis framework and the R ecosystem.src/main/resources/templates/r/readme (1)
1-6
: LGTM!The readme provides a clear description of the
matrix_to_column_list
function and outlines a comprehensive task to test the function with matrices of different shapes. This will help ensure the robustness of the function implementation.src/main/resources/templates/r/test/tests/testthat.R (1)
1-12
: LGTM!The
testthat.R
file follows the standard setup for thetestthat
testing framework in R packages. It correctly loads the necessary libraries and includes clear comments explaining the purpose and usage of the file. The code adheres to best practices for organizing tests in an R package.src/main/resources/templates/aeolus/r/default.sh (4)
4-7
: LGTM!The
install
function correctly uses theR CMD INSTALL
command to install the R package.
9-12
: LGTM!The
run_all_tests
function correctly uses theRscript
command to run tests using thetestthat
library and generate a JUnit report.
14-24
: LGTM!The
main
function correctly handles the "aeolus_sourcing" argument, changes the directory back to the initial directory, and calls theinstall
andrun_all_tests
functions.
1-26
: Reminder: Do not manually edit this file.The
default.sh
script is auto-generated by Aeolus and should not be manually edited. Changes need to be made in the Aeolus generator settings or templates.src/main/resources/templates/r/test/tests/testthat/test-convert.R (4)
1-12
: LGTM!The test case is well-structured and follows the best practice of using only one
expect_
function per test. It correctly tests the conversion of a 3x3 matrix into a list of three vectors.
14-23
: LGTM!The test case is well-structured and follows the best practice of using only one
expect_
function per test. It correctly tests the conversion of a 4x2 matrix into a list of two vectors.
25-37
: LGTM!The test case is well-structured and follows the best practice of using only one
expect_
function per test. It correctly tests the conversion of a 1x5 matrix into a list of five scalars.
39-47
: LGTM!The test case is well-structured and follows the best practice of using only one
expect_
function per test. It correctly tests the conversion of a 5x1 matrix into a list containing a single vector.src/main/resources/templates/jenkins/r/regularRuns/pipeline.groovy (1)
1-59
: LGTM!The Jenkins pipeline script for automatic grading of R assignments is well-structured and follows best practices. Here are some key observations:
- The script uses Docker to ensure a consistent and isolated environment for running tests.
- It follows a modular structure with clear separation of concerns, making it easy to maintain and extend.
- The use of JUnit XML format for reporting test results is a good choice as it integrates well with Jenkins and other CI/CD tools.
- The
postBuildTasks
function ensures that the test results are properly aggregated and cleaned up.- The script includes appropriate documentation and comments explaining its purpose and usage.
Overall, the script provides a robust and standardized way to automate the grading process for R assignments. Great job!
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2)
5-5
: LGTM!The import statement for the
R
enum value is necessary and follows the existing import style. It is placed in the correct location along with other imports from theProgrammingLanguage
enum.
29-29
: LGTM!The code segment adds a new entry to the
programmingLanguageFeatures
map for the R programming language, following the existing pattern for other languages. TheProgrammingLanguageFeature
instance is initialized with sensible default values, consistent with the entries for other languages.This change enhances the service's capability to handle R as a recognized programming language and allows for future extensions or configurations related to R in the context of GitLab CI features. It does not introduce any breaking changes or impact existing functionality.
src/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (2)
35-35
: LGTM!The change to include the R programming language in the case statement that returns the
defaultRepositoryUpgradeService
is consistent with the learning thatdefaultRepositoryUpgradeService
is used as a default for all non-JAVA languages. This change enables support for the R programming language without impacting the existing functionality.
36-36
: LGTM!The change to remove the R programming language from the list of unsupported languages that throw an
UnsupportedOperationException
is consistent with the previous change that added support for the R programming language. This ensures that the R programming language is no longer treated as an unsupported language, without impacting the existing functionality.src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
51-52
: LGTM!The addition of the
R
constant to theENABLED_LANGUAGES
set is consistent with the existing pattern and enables the R programming language in the application, aligning with the PR objective.src/main/resources/templates/gitlabci/r/regularRuns/.gitlab-ci.yml (2)
6-33
: LGTM!The
test-job
is well-structured and follows a logical sequence of steps. It uses appropriate commands and tools for cloning repositories, installing packages, and running tests. The modification of the JUnit report format is a necessary workaround for compatibility with the notification plugin. The job captures important information such as build status, commit hashes, and artifacts, which facilitates the integration of R projects into the CI/CD pipeline.
35-48
: Verify the usage of the Gradle command.The
upload-job
uses a Gradle command to run the upload process, which suggests that the notification plugin is implemented in Java. This is similar to the JavaScript CI configuration where thegradle run
command is used to operate a Java-based notification plugin, as mentioned in the provided learning.To confirm this, please run the following script to search for the usage of the
gradle run
command in the JavaScript CI configuration:Verification successful
Consistent usage of 'gradle run' command across language configurations
The 'gradle run' command is consistently used in multiple language-specific CI configurations, including R, Java (Maven), Rust, and an "empty" configuration. This suggests that the command is part of a language-agnostic process, likely related to the notification plugin as initially suspected.
- The command is present in the R configuration file:
src/main/resources/templates/gitlabci/r/regularRuns/.gitlab-ci.yml
- It's also used in Java (Maven), Rust, and a generic "empty" configuration
This consistency across different language setups supports the original implementation of the upload process using a Java-based notification plugin. The absence of a JavaScript-specific configuration file indicates that our initial assumption about a separate JavaScript CI configuration was incorrect.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `gradle run` command in the JavaScript CI configuration. # Test: Search for the `gradle run` command in the JavaScript CI configuration file. # Expect: The command is used to operate a Java-based notification plugin. rg --type yaml --ignore-case $'gradle run' src/main/resources/templates/gitlabci/javascript/regularRuns/.gitlab-ci.ymlLength of output: 235
Script:
#!/bin/bash # Description: Locate the JavaScript CI configuration file and search for the `gradle run` command. # Test 1: Find all .gitlab-ci.yml files in the src directory that might be related to JavaScript echo "Searching for JavaScript .gitlab-ci.yml files:" fd -p 'src/.*javascript.*\.gitlab-ci\.yml$' # Test 2: Search for 'gradle run' in all YAML files within the src directory echo -e "\nSearching for 'gradle run' in all YAML files:" rg --type yaml --ignore-case 'gradle run' srcLength of output: 677
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2)
9-9
: LGTM!The import statement for the
R
programming language is consistent with the existing code style and pattern. It is necessary to use theR
programming language in this class.
45-45
: Looks good!The addition of the
R
programming language to theprogrammingLanguageFeatures
map is consistent with the existing code style and pattern. It expands the service's capability to handle this programming language and enhances the extensibility of the service.Setting all feature flags to
false
indicates that theR
programming language does not support any specific features at this time, which is a reasonable default.src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts (1)
3-3
: LGTM!The addition of 'R' to the
supportedTextFileExtensions
array is a small but important part of the larger effort to introduce support for the R programming language across the application. The change is consistent with the purpose of the array and maintains its alphabetical order.src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2)
11-11
: LGTM!The import statement for the
R
programming language enum is added correctly, following the existing conventions.
52-52
: Looks good!The new entry for the
R
programming language is added correctly to theprogrammingLanguageFeatures
map. TheProgrammingLanguageFeature
instance is initialized with the appropriate parameters forR
.This change enhances the service's capability to handle programming language features for
R
, expanding its functionality.src/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
27-27
: LGTM! The addition of the R programming language aligns with the PR objectives.The new enum value
R = 'R'
is added correctly to theProgrammingLanguage
enum, following the existing naming convention. This change enhances the application's functionality by allowing it to recognize and potentially support the R programming language in contexts where programming languages are utilized, such as programming exercises or assessments.Please ensure that corresponding updates are made in other relevant parts of the codebase to fully integrate R programming exercises, such as:
- Updating documentation and user interfaces to include R as a supported language option.
- Implementing necessary backend services and configurations to handle R-based exercises.
- Adding appropriate test cases to verify the functionality of R programming exercises.
src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (3)
222-223
: LGTM!The addition of the
R
programming language case is consistent with the existing behavior for other supported languages. It indicates thatR
is now supported for assignments and the assignment repository should be checked out to the "assignment" subdirectory.
233-235
: Looks good!The addition of the
R
programming language case aligns with the existing logic for test repositories. Returning an empty string forR
indicates that the test repository should be checked out to the root directory, which is consistent with the behavior for other languages likeJAVA
,PYTHON
, etc.
245-247
: Looks good to me!The addition of the
R
programming language case follows the pattern for solution repositories. Returning "solution" forR
indicates that the solution repository should be checked out to the "solution" subdirectory, which aligns with the behavior for other languages likeHASKELL
andOCAML
.docs/user/exercises/programming-exercise-features.inc (2)
38-39
: LGTM!The addition of R to the programming language support table is consistent with the PR objective. The table structure and formatting are maintained.
72-73
: Looks good!The addition of the R programming language to the feature support table is consistent with the PR objective. The supported features for R are appropriately marked, similar to Rust. The table structure and formatting are maintained.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java (1)
187-188
: LGTM!The code changes add support for the
R
programming language in thebuilderFor
method, which is consistent with the list of alterations provided in the summary. The implementation looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range comments (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Line range hint
1-33
: Consider the impact of adding features to a deprecated class.While the addition of R language support is implemented correctly, it's worth noting that this class is marked as deprecated and scheduled for removal in version 8.0.0. Adding new features to a deprecated class may lead to maintenance issues in the future.
Consider the following alternatives:
- If R support is crucial before the removal of GitLab CI, proceed with caution and ensure proper migration documentation.
- If possible, implement R support directly in the new system that will replace GitLab CI (mentioned as LocalVC in the comment).
- Create a separate, non-deprecated class for R support that can be easily migrated or integrated into the new system.
Please clarify the strategy for handling this addition in light of the planned deprecation.
build.gradle (1)
Line range hint
1-1010
: Recommendation for testing and documentation.The addition of R language support appears to be correctly implemented in the
build.gradle
file. To ensure a smooth integration, please:
- Run a full build and test cycle to verify that the R language dependency doesn't introduce any conflicts or issues.
- Update the project's documentation, including the README and any developer guides, to mention the new R language support.
- If there are any specific setup or configuration steps required for R language support, add them to the project's setup instructions.
These steps will help maintain the project's stability and keep all developers informed about the new language support.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (6)
- build.gradle (1 hunks)
- docs/user/exercises/programming-exercise-features.inc (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
📓 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/gitlabci/GitLabCIProgrammingLanguageFeatureService.java:24-24 Timestamp: 2024-06-29T14:49:34.001Z Learning: Static code analysis for Rust in GitLab CI will be added in a follow-up PR.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/jenkins/JenkinsProgrammingLanguageFeatureService.java:43-43 Timestamp: 2024-06-29T14:47:20.796Z Learning: Jenkins support for Rust was added in commit d0fb7032e70c001dc8b2c4d0a391ca8bf7e7d586.
🔇 Additional comments (8)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
6-6
: LGTM: Import statement for R language added correctly.The static import for the R programming language is consistent with other language imports and follows the coding guidelines.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
10-10
: LGTM: Import statement for R is correctly added.The import statement for R is properly placed with other similar imports and follows the coding guideline to avoid star imports.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
55-55
: LGTM: R language support correctly added.The addition of R programming language support is implemented correctly and consistently with other language entries. It adheres to the coding guidelines, including proper naming conventions and principles. The structure matches the existing pattern, which promotes maintainability.
Regarding the past review suggestion about adding comments for readability, I acknowledge the author's response that most IDEs can show parameter names. If this is a standard practice in your team and doesn't hinder understanding for new team members, then the current implementation is acceptable.
docs/user/exercises/programming-exercise-features.inc (1)
40-41
: LGTM: R language support added correctly.The addition of R to the programming language support table is consistent with the existing structure and aligns with the PR objectives.
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2)
40-40
: Import statement for R language support added correctly.The import statement for
de.jplag.rlang.RLanguage
is appropriately placed with other JPlag language imports and follows Java naming conventions.
40-40
: R language support successfully integrated into the plagiarism detection service.The changes effectively introduce support for the R programming language in the
ProgrammingPlagiarismDetectionService
class. The import statement and the modification to thegetJPlagProgrammingLanguage
method are consistent with the existing code structure and style. These changes align well with the PR objectives of integrating R language support into the Artemis platform.Also applies to: 321-322
build.gradle (2)
Line range hint
252-262
: Verify complete R language integration in build process.The R language dependency has been added correctly among other JPlag language dependencies. To ensure full integration, please verify the following:
- Check if any task definitions or custom configurations in the build file need to be updated to include R language support.
- Verify that any language-specific plugins or tools used in the build process are compatible with R language files.
- If there are any test configurations, ensure they cover R language files as well.
These steps will help guarantee that R language support is fully integrated into all aspects of the build and test process.
To check for any R-specific configurations or tasks, you can run:
#!/bin/bash # Search for any existing R-related configurations or tasks grep -n -i "r.*lang" build.gradle
257-257
: R language support added successfully.The addition of the R language dependency
implementation "de.jplag:rlang:${jplag_version}"
is consistent with the other language dependencies in this section. This change successfully integrates R language support into the project.However, to ensure complete integration, consider the following:
- Verify that
jplag_version
is defined in the project'sgradle.properties
file or elsewhere in the build script.- Check if any additional configurations or plugins are needed to fully support R language processing in your project.
- Update any relevant documentation or README files to mention the new R language support.
To confirm the
jplag_version
is properly defined, you can run:✅ Verification successful
To accurately verify the definition and value of
jplag_version
, please run the following shell script:
jplag_version
is defined correctly.The
jplag_version
is set to5.1.0
ingradle.properties
, ensuring that the R language dependency uses the specified version.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if jplag_version is defined in gradle.properties grep -q "jplag_version" gradle.properties && echo "jplag_version is defined" || echo "jplag_version is not defined" # Display the value of jplag_version if it exists grep "jplag_version" gradle.propertiesLength of output: 297
Script:
#!/bin/bash # Check if jplag_version is defined in gradle.properties if grep -q "^jplag_version=" gradle.properties; then echo "jplag_version is defined." # Display the value of jplag_version grep "^jplag_version=" gradle.properties else echo "jplag_version is not defined." fiLength of output: 169
...um/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java
Outdated
Show resolved
Hide resolved
...in/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java
Outdated
Show resolved
Hide resolved
This should reduce merge conflicts by reducing changes to adjacent lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Line range hint
20-21
: Consider the impact of adding features to a deprecated class.While the additions for R language support and updates to Rust are valid, it's worth noting that this class is marked as deprecated.
Given the deprecation notice and the recommendation to migrate to LocalVC, consider the following:
- Evaluate if these changes should also be implemented in the new LocalVC system.
- Ensure that the migration guide or documentation is updated to reflect these new language supports.
- Consider adding a comment explaining why new features are being added to a deprecated class, if this is a deliberate decision.
This will help maintain consistency and smooth the transition for users migrating from GitLab CI to LocalVC.
Also applies to: 30-31
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (7)
- build.gradle (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/webapp/app/entities/programming/programming-exercise.model.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
📓 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/gitlabci/GitLabCIProgrammingLanguageFeatureService.java:24-24 Timestamp: 2024-06-29T14:49:34.001Z Learning: Static code analysis for Rust in GitLab CI will be added in a follow-up PR.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/jenkins/JenkinsProgrammingLanguageFeatureService.java:43-43 Timestamp: 2024-06-29T14:47:20.796Z Learning: Jenkins support for Rust was added in commit d0fb7032e70c001dc8b2c4d0a391ca8bf7e7d586.
🔇 Additional comments (14)
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
Line range hint
15-15
: LGTM: R language added correctlyThe addition of the R language to the
ProgrammingLanguage
enum is correct and consistent with the existing structure. The file extension "R" is appropriate for R language files.src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2)
6-6
: LGTM: R language import added correctly.The import statement for the R language is correctly placed and follows the coding guideline of avoiding star imports.
31-31
: LGTM: Rust language support updated. Clarification needed on static code analysis.The update to Rust language support is consistent with the R language entry. However, I noticed that the static code analysis flag is set to true for Rust.
Could you please clarify if this is intentional? There's a note mentioning that static code analysis for Rust will be added in a follow-up PR, but it appears to be already enabled here.
src/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
16-28
: 🧹 Nitpick (assertive)Consider sorting enum entries alphabetically for better maintainability.
The addition of 'R' to the
ProgrammingLanguage
enum aligns with the PR objectives. However, the current ordering of entries doesn't follow a clear pattern, which may impact readability and maintainability.To improve this, consider sorting the enum entries alphabetically. This would make it easier to locate specific entries and maintain consistency as new languages are added in the future.
Here's a suggested alphabetical order:
export enum ProgrammingLanguage { ASSEMBLER = 'ASSEMBLER', C = 'C', EMPTY = 'EMPTY', HASKELL = 'HASKELL', JAVA = 'JAVA', JAVASCRIPT = 'JAVASCRIPT', KOTLIN = 'KOTLIN', OCAML = 'OCAML', PYTHON = 'PYTHON', R = 'R', RUST = 'RUST', SWIFT = 'SWIFT', VHDL = 'VHDL', }This change would enhance code readability while maintaining the intended functionality.
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2)
40-40
: Import for R language support added correctly.The import statement for
de.jplag.rlang.RLanguage
is correctly placed and consistent with other language imports. This addition is necessary for supporting R language in the plagiarism detection service.
Line range hint
1-524
: R language support successfully integrated into the plagiarism detection service.The changes made to this file successfully integrate support for the R programming language in the plagiarism detection service. The modifications are consistent with the existing code structure and style, and no further changes appear necessary for R language support in this file.
These changes align well with the PR objectives of integrating R programming language into the Artemis platform, responding to requests from students at the University Innsbruck.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (5)
10-10
: Import statement for R added correctly.The import statement for
R
has been added appropriately.
37-38
: Verify the inclusion of C and HASKELL entries.The entries for
C
andHASKELL
are present in theprogrammingLanguageFeatures
map. Please verify whether reintroducing these programming languages is intentional, as they appear to have been removed previously.
41-41
: Verify the inclusion of JAVASCRIPT entry.The
JAVASCRIPT
entry is included in theprogrammingLanguageFeatures
map. Confirm if this addition is intended.
44-44
: Addition of R programming language support is implemented correctly.The
R
programming language has been correctly added to theprogrammingLanguageFeatures
map with appropriate feature flags.
45-45
: Verify the inclusion of RUST entry.The
RUST
programming language entry is included. Please confirm if reintroducingRUST
is intentional.src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (3)
13-13
: Correctly imported the R programming languageThe import statement for
ProgrammingLanguage.R
on line 13 is properly added, enabling the use of the R language in the service.
43-45
: Verify the re-insertion of language entriesLines 43-45 reintroduce entries for
ASSEMBLER
,C
, andHASKELL
into theprogrammingLanguageFeatures
map. Please confirm that this re-addition is intentional and that their configurations match the expected feature sets.
52-52
: R programming language features configured correctlyLine 52 adds the R programming language to the
programmingLanguageFeatures
map with the following settings:
hasFormatter
:false
hasStaticCodeAnalysis
:false
hasTestCoverage
:true
hasInlineCodeEditing
:false
hasInlineErrorFeedback
:false
supportedProjectTypes
:List.of()
supportsDebugging
:false
supportsIncrementalBuild
:true
These configurations appear appropriate for the R language within the local CI environment.
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java
Show resolved
Hide resolved
.../cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java
Outdated
Show resolved
Hide resolved
...in/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java
Show resolved
Hide resolved
...um/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range comments (1)
build.gradle (1)
Line range hint
1-1010
: Consider enhancing dependency management and organization.While the current build file is functional, there are a couple of suggestions that could improve maintainability and readability:
Consider using a dependency management plugin like "com.github.ben-manes.versions" more extensively. This can help in keeping dependencies up-to-date and managing versions more effectively.
Group similar dependencies together. For example, all Spring-related dependencies could be grouped, all testing dependencies could be in another group, etc. This can improve readability and make it easier to manage dependencies in the future.
Would you like me to provide an example of how to implement these suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested on ts6, doesnt work with gitlabci (should this be supported)
Tested locally with localci, works as expected
71f01fe
Checklist
General
Server
Changes affecting Programming Exercises
Motivation and Context
Students from the University Innsbruck requested support for integrating the R language into Artemis.
Description
This PR adds the R programming language as an exercise template. It is structured according to R Packages (2e) and uses
testthat
as the testing framework. Thanks to Johanna and Marco for providing an exercise.Steps for Testing
Note that GitLab CI is not supported.
Prerequisites:
R
languageTemplate Result
should show 0 passed tests and no build failureSolution Result
should show 4 passed tests and no build failureTestserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Enhancements
Bug Fixes